Skip to content

vecgeom: Do not read destructed memory#9719

Merged
smuzaffar merged 2 commits intoIB/CMSSW_15_1_X/masterfrom
vecgeom-VECGEOM-624
Feb 25, 2025
Merged

vecgeom: Do not read destructed memory#9719
smuzaffar merged 2 commits intoIB/CMSSW_15_1_X/masterfrom
vecgeom-VECGEOM-624

Conversation

@smuzaffar
Copy link
Contributor

@smuzaffar smuzaffar commented Feb 25, 2025 •
edited
Loading

Apply changes from https://gitlab.cern.ch/VecGeom/VecGeom/-/merge_requests/1262

FYI @civanch
We do get these warnings for GCC 14 builds https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/el8_amd64_gcc14/CMSSW_15_1_X_2025-02-24-1900/BigProducts/Simulation

  vecgeom/v1.2.10-73ad1151de5c4a9ee0003bb695ac82c0/include/VecGeom/base/VariableSizeObj.h:182:32: warning: 'MEM[(struct VariableData_t &)_6 + 4].fSelfAlloc' may be used uninitialized [-Wmaybe-uninitialized]
   182 |     if (obj->GetVariableData().fSelfAlloc) delete[](char *) obj;
      |                                ^
In function 'ReleaseInstance',
    inlined from '__dt_base ' at vecgeom/v1.2.10-73ad1151de5c4a9ee0003bb695ac82c0/include/VecGeom/volumes/TessellatedStruct.h:229:43,
    inlined from '__dt_base ' at vecgeom/v1.2.10-73ad1151de5c4a9ee0003bb695ac82c0/include/VecGeom/volumes/UnplacedTessellated.h:38:7,
    inlined from '__dt_base ' at geant4/11.2.2-cb96bd8c2ff4d6e5ba67de349ddad4cb/geant4.11.2.2/source/geometry/management/include/G4UAdapter.hh:264:1,
    inlined from '__dt_base ' at geant4/11.2.2-cb96bd8c2ff4d6e5ba67de349ddad4cb/geant4.11.2.2/source/geometry/solids/specific/src/G4UTessellatedSolid.cc:76:1:
  vecgeom/v1.2.10-73ad1151de5c4a9ee0003bb695ac82c0/include/VecGeom/base/VariableSizeObj.h:182:32: warning: 'MEM[(struct VariableData_t &)_57 + 4].fSelfAlloc' may be used uninitialized [-Wmaybe-uninitialized]
   182 |     if (obj->GetVariableData().fSelfAlloc) delete[](char *) obj;

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 25, 2025

A new Pull Request was created by @smuzaffar for branch IB/CMSSW_15_1_X/master.

@cmsbuild, @iarspider, @smuzaffar can you please review it and eventually sign? Thanks.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.
cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 25, 2025 •
edited
Loading

cms-bot internal usage

@smuzaffar
Copy link
Contributor Author

smuzaffar commented Feb 25, 2025

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 25, 2025

Pull request #9719 was updated.

@smuzaffar
Copy link
Contributor Author

smuzaffar commented Feb 25, 2025

please test

@civanch
Copy link
Contributor

civanch commented Feb 25, 2025

@smuzaffar , this fix needs to be integrated. If I understand correctly it is essential for phase2.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 25, 2025

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4cac46/44636/summary.html
COMMIT: cb58ba9
CMSSW: CMSSW_15_1_X_2025-02-25-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/9719/44636/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4cac46/44636/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4cac46/44636/git-merge-result

Comparison Summary

Summary:

  • You potentially added 18 lines to the logs
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3920300
  • DQMHistoTests: Total failures: 21
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3920259
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 214 log files, 184 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@smuzaffar
Copy link
Contributor Author

smuzaffar commented Feb 25, 2025

+externals

@smuzaffar smuzaffar merged commit a0ab18a into IB/CMSSW_15_1_X/master Feb 25, 2025
9 checks passed
@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 25, 2025

This pull request is fully signed and it will be integrated in one of the next IB/CMSSW_15_1_X/master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @rappoccio, @sextonkennedy, @antoniovilela, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2)

This was referenced Feb 26, 2025
Closed
Merged
@cmsbuild cmsbuild mentioned this pull request Feb 28, 2025
Merged
@smuzaffar smuzaffar deleted the vecgeom-VECGEOM-624 branch March 28, 2025 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

externals-approved fully-signed orp-pending tests-approved

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@smuzaffar @cmsbuild @civanch